Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

support add/delete User manager my sql #1120

Merged
merged 47 commits into from
Jun 8, 2020
Merged

support add/delete User manager my sql #1120

merged 47 commits into from
Jun 8, 2020

Conversation

buhongw7583c
Copy link
Contributor

A new PR related with original one:
#1101

Closes #1101

What this PR does / why we need it:
All the PR description and comments could be found from #1101.
Due to some technical issue, the original PR could not display the correct changed files and have to reopen a new PR.

Special notes for your reviewer:
All the PR description and comments could be found from #1101.

How does this PR make you feel:
gif

If applicable:

  • this PR contains documentation
  • this PR contains tests

@buhongw7583c buhongw7583c linked an issue Jun 1, 2020 that may be closed by this pull request
As an operator, I can use the service operator to manage a user within the PostgreSQL database #1056
Closed
3 tasks
@buhongw7583c buhongw7583c removed a link to an issue Jun 1, 2020
As an operator, I can use the service operator to manage a user within the PostgreSQL database #1056
Closed
3 tasks
@buhongw7583c buhongw7583c linked an issue Jun 1, 2020 that may be closed by this pull request
As an operator, I can use the Service operator to manage the users within MySQL database #1055
Closed
3 tasks
@buhongw7583c
Copy link
Contributor Author

buhongw7583c commented Jun 1, 2020 •
edited
Loading

@melonrush13 Mel, I had some technical issue with the original PR that the changed files are wrong so have to open an exactly new one but this time with the correct changed files list.
Now the mysqluser will be deleted when the IP address has no access to server. As you suggested, let's do the same handling with the AzureSqlUser issue #1112.

melonrush13
melonrush13 reviewed Jun 1, 2020
View reviewed changes
PROJECT Show resolved Hide resolved
melonrush13
melonrush13 suggested changes Jun 1, 2020
View reviewed changes
Copy link
Contributor

@melonrush13 melonrush13 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was able to...

  • Create the user and delete the user successfully
  • Create/Delete with keyvault set from adminSecret/adminSecretKeyVault successfully
  • Create/Delete with keyVaultToStoreSecrets successfully

Quick fix, Hong: Please address PROJECT file for accidentantal double declaration. Thanks!

Also - as for the other PR, please add documentation to the following file for this new operator, but also feel free to address in another PR!

We will address the improper connected IP (for firewallrule) in this ticket here

Tagging @frodopwns @jananivMS for additional eyes/comments

buhongw7583c and others added 8 commits June 2, 2020 21:46
@buhongw7583c
Merge branch 'UserManagerMySQL' of https://github.com/buhongw7583c/az…
868b90a 
…ure-service-operator-1 into UserManagerMySQL
@buhongw7583c
changes per comments: modify the cert verion in cainjection, change t…
e5e594b 
…o v2 for mysqlserver in ctonrooler, move the common function to stringhelper, use the secret to derive the full server name, update the user secret with the right mysqldnssuffix.
@frodopwns
Merge branch 'master' into UserManagerMySQL
c4134c5
@buhongw7583c
update project file
f6b9689
@buhongw7583c
go mod, go.sum. Remove unneccsary user defined format secret. Small c…
22dedce 
…hanged etc.
@buhongw7583c
Merge branch 'UserManagerMySQL' of https://github.com/buhongw7583c/az…
03bf8cf 
…ure-service-operator-1 into UserManagerMySQL
@buhongw7583c
Merge branch 'master' into UserManagerMySQL
006b31b
@buhongw7583c
Merge branch 'master' into UserManagerMySQL
3ea55b9
@buhongw7583c
Copy link
Contributor Author

@melonrush13 @frodopwns Mel and Erin, I think I need at least one of you to approve the PR? Thanks so much.

melonrush13
melonrush13 reviewed Jun 5, 2020
View reviewed changes
PROJECT Show resolved Hide resolved
jananivMS and others added 3 commits June 6, 2020 21:51
@jananivMS @davefellows
Update docs/services/mysql/mysql.md
b4d4df2 
Co-authored-by: Dave Fellows <dave.fellows@microsoft.com>
@jananivMS @davefellows
Update docs/services/mysql/mysql.md
5dd2ad7 
Co-authored-by: Dave Fellows <dave.fellows@microsoft.com>
@jananivMS @davefellows
Update pkg/resourcemanager/mysql/mysqluser/mysqluser_reconcile.go
9d60f74 
Co-authored-by: Dave Fellows <dave.fellows@microsoft.com>
@jananivMS jananivMS self-requested a review June 7, 2020 03:58
jananivMS and others added 2 commits June 7, 2020 20:46
@jananivMS
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

jananivMS
jananivMS approved these changes Jun 8, 2020
View reviewed changes
Copy link
Contributor

@jananivMS jananivMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Tested functionality using Helm and works.

@jananivMS
Copy link
Contributor

/azp run

@buhongw7583c buhongw7583c merged commit 725dc08 into Azure:master Jun 8, 2020
@azure-pipelines
Copy link

Pipelines were unable to run due to time out waiting for the pull request to finish merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davefellows davefellows davefellows left review comments

@melonrush13 melonrush13 melonrush13 left review comments

@jananivMS jananivMS jananivMS approved these changes

@frodopwns frodopwns Awaiting requested review from frodopwns

@WilliamMortlMicrosoft WilliamMortlMicrosoft Awaiting requested review from WilliamMortlMicrosoft

Assignees

@jananivMS jananivMS

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

As an operator, I can use the Service operator to manage the users within MySQL database
5 participants
@buhongw7583c @jananivMS @davefellows @melonrush13 @frodopwns